-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
incr. comp.: Use more bits for DefPath hashes #42082
incr. comp.: Use more bits for DefPath hashes #42082
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ |
📌 Commit 4549423 has been approved by |
…hes, r=eddyb incr. comp.: Use more bits for DefPath hashes Use 128 instead of 64 bits for DefPath hashes, like we do for everything else. Collision probability is unnecessarily high with 64 bits. Also change the representation of `ich::Fingerprint` from `Fingerprint([u8; 16])` to `Fingerprint(u64, u64)` which is better for hashers like `FxHasher`.
…hes, r=eddyb incr. comp.: Use more bits for DefPath hashes Use 128 instead of 64 bits for DefPath hashes, like we do for everything else. Collision probability is unnecessarily high with 64 bits. Also change the representation of `ich::Fingerprint` from `Fingerprint([u8; 16])` to `Fingerprint(u64, u64)` which is better for hashers like `FxHasher`.
⌛ Testing commit 4549423 with merge 92b0b81... |
💔 Test failed - status-appveyor |
…hes, r=eddyb incr. comp.: Use more bits for DefPath hashes Use 128 instead of 64 bits for DefPath hashes, like we do for everything else. Collision probability is unnecessarily high with 64 bits. Also change the representation of `ich::Fingerprint` from `Fingerprint([u8; 16])` to `Fingerprint(u64, u64)` which is better for hashers like `FxHasher`.
Out of curiosity, why didn't this just use |
@pthariensflame Mostly because our hasher works with usize, so using |
What are the platforms where |
…oerister Don't byteswap Fingerprints when encoding Byteswapping Fingerprints when encoding is unnessesary and breaks if the Fingerprint is later decoded on a machine with different endianness to the one it was encoded on. Fixes rust-lang#42239 This PR fixes a regression caused by rust-lang#42082. @michaelwoerister
…oerister Don't byteswap Fingerprints when encoding Byteswapping Fingerprints when encoding is unnessesary and breaks if the Fingerprint is later decoded on a machine with different endianness to the one it was encoded on. Fixes rust-lang#42239 This PR fixes a regression caused by rust-lang#42082. @michaelwoerister
…oerister Don't byteswap Fingerprints when encoding Byteswapping Fingerprints when encoding is unnessesary and breaks if the Fingerprint is later decoded on a machine with different endianness to the one it was encoded on. Fixes rust-lang#42239 This PR fixes a regression caused by rust-lang#42082. @michaelwoerister
…oerister Don't byteswap Fingerprints when encoding Byteswapping Fingerprints when encoding is unnessesary and breaks if the Fingerprint is later decoded on a machine with different endianness to the one it was encoded on. Fixes rust-lang#42239 This PR fixes a regression caused by rust-lang#42082. @michaelwoerister
Use 128 instead of 64 bits for DefPath hashes, like we do for everything else. Collision probability is unnecessarily high with 64 bits.
Also change the representation of
ich::Fingerprint
fromFingerprint([u8; 16])
toFingerprint(u64, u64)
which is better for hashers likeFxHasher
.